ready page#4476
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am approving your solution because all core functional requirements are met: App is decomposed into Welcome, Header, and Article, each in its own folder with default exports and co-located CSS, and the app will work correctly. The remaining issues are style-related and do not block functionality, such as the overuse of blank lines in App.jsx, Header.css, and Welcome.css, which touches [CHECKLIST ITEM #1] but is minor and easily fixable with a quick formatting pass. Another small point is that Article.jsx uses className="article__title" without a corresponding .article__title rule in Article.css; consider adding a simple rule there or reusing an existing class so the title styling matches your decomposed CSS structure. If you decide to further improve the decomposition, you can optionally extract the <nav> block into a dedicated Navigation component with its own stylesheet, but this is not strictly required for this task and does not block approval. Overall, your component structure, default exports, and JSX are solid—just tighten the CSS spacing and class-name alignment as you iterate on future tasks.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| function App() { | ||
| return ( | ||
| <main className="app"> |
There was a problem hiding this comment.
This extra blank line contributes to the pattern described in checklist item #1: "[CODE STYLE] - Don't overuse empty lines between components.". Consider tightening up unnecessary empty lines between rules to avoid matching the BAD example.
| <Welcome /> | ||
| <Header /> | ||
| <Article /> | ||
| </main> |
There was a problem hiding this comment.
Another superfluous blank line between related declarations; together these match the overuse pattern described in checklist item #1. Try to keep only necessary spacing between blocks.
| function Article() { | ||
| return ( | ||
| <article className="article"> | ||
| <h2 className="article__title">Headline</h2> |
There was a problem hiding this comment.
This violates checklist item #1: Don't overuse empty lines between components. Your JSX currently has a blank line between component usages, which matches the BAD example pattern.
| padding: 1em; | ||
| font-size: 140%; | ||
| position: sticky; | ||
| top: 0; |
There was a problem hiding this comment.
This extra blank line between imports and the App component matches the "BAD EXAMPLE" from checklist item #1: Don't overuse empty lines between components. Consider removing it so imports and the component definition are grouped without an extra separator.
| padding: 1em; | ||
| font-size: 140%; | ||
| position: sticky; | ||
| top: 0; |
There was a problem hiding this comment.
You render className="article__title" here but Article.css does not define a .article__title rule. Either add a corresponding selector in Article.css or reuse another existing class so the title styling is consistent with the decomposed CSS block.
| // Create a Welcome function returning the HTML of welcome block | ||
| function Welcome() { | ||
| return ( | ||
| <section className="welcome"> |
There was a problem hiding this comment.
There is an unnecessary blank line between the .welcome rule and .welcome__text, which contributes to the overuse pattern described in checklist item #1: Don't overuse empty lines between components. Consider removing it to keep rule blocks tightly grouped.
2 participants
https://MarianaKlochnyk.github.io/react_decompose/